-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: always include mode instructions regardless of Power Steering setting #7279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tting - Mode roleDefinition and customInstructions are now always included in environment details - Previously these were only included when Power Steering was enabled - Updated tests to verify mode instructions are included in both cases Fixes #7277
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backward but the bugs are still mine.
| details += `<slug>${currentMode}</slug>\n` | ||
| details += `<name>${modeDetails.name}</name>\n` | ||
| details += `<model>${modelId}</model>\n` | ||
| details += `<role>${modeDetails.roleDefinition}</role>\n` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean fix! This correctly ensures mode instructions are always included. Though after this change, you might want to check if the EXPERIMENT_IDS import is still needed in this file since we're no longer checking for POWER_STEERING.
| }) | ||
|
|
||
| it("should include experiment-specific details when Power Steering is enabled", async () => { | ||
| it("should always include mode role and custom instructions regardless of Power Steering", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test coverage for both scenarios! The test name clearly indicates the behavior we're verifying - that mode instructions should always be included regardless of experimental features.
|
I don't know the code base all that well, does this fix only effect the first instructions send to the LLM or all instructions sent to the LLM? I thought Power Steering was used to occasionally include more details (not always) but the first instructions sent should always include all the environment details in the instruction. I'm not asking the BOT to make code changes, but to make sure the proposed change does what I thought the intent and purpose of power steering was. |
Related GitHub Issue
Closes: #7277
Roo Code Task Context (Optional)
This PR was created with assistance from Roo Code to address the reported issue.
Description
This PR fixes an issue where mode instructions and rules were not being included in the environment details when the Power Steering feature was disabled.
Key changes:
roleDefinitionandcustomInstructionswhen Power Steering was enabledThe implementation is minimal and focused - it simply removes the unnecessary conditional wrapper around the mode instruction inclusion logic.
Test Procedure
Testing performed:
cd src && npx vitest run core/environment/__tests__/getEnvironmentDetails.spec.tsTo verify the fix:
Pre-Submission Checklist
Screenshots / Videos
Not applicable - this is a backend fix that affects how mode instructions are included in prompts.
Documentation Updates
This is a bug fix that restores expected behavior. The mode instructions should always be included regardless of experimental features.
Additional Notes
This was a straightforward fix that addresses the core issue reported by the user. The mode instructions are essential context for the LLM to understand what is expected of each mode, so they should not be gated behind an experimental feature flag.
Get in Touch
Available for any questions about this PR.
Important
Fixes issue where mode instructions were not included when Power Steering was disabled by always including them in
getEnvironmentDetails.ts.getEnvironmentDetails.tsto always includeroleDefinitionandcustomInstructionsin environment details, regardless of Power Steering status.getEnvironmentDetails.spec.tsto verify mode instructions are included with Power Steering both enabled and disabled.This description was created by
for 6f4f2c0. You can customize this summary. It will automatically update as commits are pushed.